Repository navigation
Fiber: hold the object a callable resolved to in the creating frame - #24134
Conversation
Inside a method of A, new Fiber([A::class, 'm']) or new Fiber("A::m")
resolves the callable to the method's $this. Fiber::__construct() kept
that object in fci.object and fci_cache.object without a reference, so
once the A died before start(), the fiber called A::m() on freed memory.
The fiber now holds fci.object for every callable, releases it with the
callable (after the call, or when the fiber is freed unstarted) and
reports it to the GC, so a fiber stored in a property of that object is
a collectable cycle.
A __call trampoline is not covered: it is resolved again at start(),
against the frame that calls start().
Done. Thank you |
|
Seems there is a conflict, mind rebasing? |
@Girgias |
GitHub was showing it as having a conflict and thus non-mergeable.... so I guess it was just drunk. |
* PHP-8.6:
Update NEWS
ext/standard/tests: work around Windows quoting with tempfiles.
.github/scripts/windows: pass an extension_dir to the test runner.
{ext/standard,sapi/cli}/tests: pass TEST_PHP_EXTRA_ARGS to php
ext/standard/tests: pass TEST_PHP_EXTRA_ARGS to some proc_open() tests
sapi/cli/tests/023.phpt: support TEST_PHP_EXTRA_ARGS
std: Fix memory leak of the last response headers after a nested HTTP request (#24201)
Fiber: hold the object a callable resolved to in the creating frame (#24134)
* PHP-8.4:
Update NEWS
ext/standard/tests: work around Windows quoting with tempfiles.
.github/scripts/windows: pass an extension_dir to the test runner.
{ext/standard,sapi/cli}/tests: pass TEST_PHP_EXTRA_ARGS to php
ext/standard/tests: pass TEST_PHP_EXTRA_ARGS to some proc_open() tests
sapi/cli/tests/023.phpt: support TEST_PHP_EXTRA_ARGS
std: Fix memory leak of the last response headers after a nested HTTP request (php#24201)
Fiber: hold the object a callable resolved to in the creating frame (php#24134)
* PHP-8.5:
Update NEWS
ext/standard/tests: work around Windows quoting with tempfiles.
.github/scripts/windows: pass an extension_dir to the test runner.
{ext/standard,sapi/cli}/tests: pass TEST_PHP_EXTRA_ARGS to php
ext/standard/tests: pass TEST_PHP_EXTRA_ARGS to some proc_open() tests
sapi/cli/tests/023.phpt: support TEST_PHP_EXTRA_ARGS
std: Fix memory leak of the last response headers after a nested HTTP request (php#24201)
Fiber: hold the object a callable resolved to in the creating frame (php#24134)
Fix use-after-free when a Fiber callable resolves to the creating method's $this
Problem
Inside a method of a class,
[A::class, 'm']and"A::m"resolve to a call ofm()on the current$this.Fiber::__construct()stores that object infci.objectandfci_cache.objectbut takes no reference to it. If the object dies beforeFiber::start(), the fiber calls the method on freed memory.PHP 8.3.6 and PHP-8.4 print
releasedbeforem on a1: the destructor has run and the object is freed whenm()executes. Valgrind reports invalid reads. A new object allocated betweenmake()andstart()can take the freed slot.Cause
Fiber::__construct()(Zend/zend_fibers.c) keeps a reference only to the callable zval:For
[A::class, 'm']the zval holds a class name and a method name, not the object. The object comes from the calling frame's$this, so nothing keeps it alive.On PHP-8.4 a fiber suspended inside
m()hits the same bug: if the last reference to the object goes while the fiber is suspended,m()resumes on freed memory.Fix
Fiber::__construct()adds a reference tofci.objectwhenever it is set, for every kind of callable, so the release and the GC report need no condition.zend_fiber_release_callable()releases the callable zval and that object together, after the call returns or when an unstarted fiber is freed.zend_fiber_object_gc()reports the object, so a fiber stored in a property of the same object is a collectable cycle.Not covered: a callable that resolves to
__callis resolved again atstart(), against the frame that callsstart(). That is a separate issue.Tests
Zend/tests/fibers/fiber-callable-object-outlives-caller.phptcovers five cases:"A::m";gc_collect_cycles()collects.Verified on a
--disable-all --enable-debugbuild of PHP-8.4:Zend/tests/fiberspass and the new test is clean under valgrind (run-tests.php -m).The same change on master passes the 111 tests in
Zend/tests/fibers.